Skip to content

Retry "use of closed network connection" in s3 backup handle#6182

Merged
deepthi merged 4 commits intovitessio:masterfrom
tinyspeck:am_retry_s3_connclosed
May 15, 2020
Merged

Retry "use of closed network connection" in s3 backup handle#6182
deepthi merged 4 commits intovitessio:masterfrom
tinyspeck:am_retry_s3_connclosed

Conversation

@ajm188
Copy link
Copy Markdown
Contributor

@ajm188 ajm188 commented May 14, 2020

This address some of the issues with taking backups with s3backupstorage after upgrade to aws-sdk-go version 1.28.x.

The approach is to create a custom retryer to check for errors that match the "closed network connection" error string.

When creating the retryer, embed aws's DefaultRetryer (which is the logic that gets used if you don't specify anything), and use this to determine the rest of the retryable errors, as well as the backoff logic.

@ajm188 ajm188 requested a review from sougou as a code owner May 14, 2020 17:38
@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented May 14, 2020

hi @ajm188! can you fix the DCO?

Andrew Mason added 3 commits May 14, 2020 14:05
…aking s3 backups

Signed-off-by: Andrew Mason <amason@slack-corp.com>
The error likely gets thrown
[here](https://github.com/aws/aws-sdk-go/blob/master/aws/corehandlers/handlers.go#L164),
so it seems it would return "send request failed", and the original
error message won't be in `r.Error.Error()` but rather in the
`OrigErr()` of the `awserr.Error`.

So in order to match the closed connection error, we will need to check
if `r.Error` is an `awserr.Error` and then check the `OrigErr()`

Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 force-pushed the am_retry_s3_connclosed branch from adb58d0 to 3dc147c Compare May 14, 2020 18:06
"github.com/aws/aws-sdk-go/aws/request"
)

type ClosedConnectionRetryer struct {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this struct satisfies a particular interface. That should be documented.

Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 force-pushed the am_retry_s3_connclosed branch from 30ea25d to 5bcf1c6 Compare May 15, 2020 01:01
Copy link
Copy Markdown
Collaborator

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@deepthi deepthi merged commit 5dc35b0 into vitessio:master May 15, 2020
morgo added a commit to planetscale/vitess that referenced this pull request May 19, 2020
Signed-off-by: Morgan Tocker <tocker@gmail.com>
morgo added a commit that referenced this pull request May 19, 2020
@ajm188 ajm188 deleted the am_retry_s3_connclosed branch June 23, 2020 21:44
@deepthi deepthi added this to the v7.0 milestone Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants